Skip to content

fix: return zero rates instead of NaN for zero-rate compressor timesteps#1535

Open
olelod wants to merge 2 commits intomainfrom
fix/zero-rate-actual-rate-ffill
Open

fix: return zero rates instead of NaN for zero-rate compressor timesteps#1535
olelod wants to merge 2 commits intomainfrom
fix/zero-rate-actual-rate-ffill

Conversation

@olelod
Copy link
Copy Markdown
Contributor

@olelod olelod commented May 4, 2026

Type of Work

  • Patch: X.Y.Z+1. NEGLIGIBLE visible changes, does not change input or output - OR changes behaviour. Use chore:, refactor: etc
  • Minor: X.Y+1.Z. Minor changes, might ADD new input (YAML), or other backwards-compatible changes. Use feat:, fix:
  • Major: X+1.Y.Z. Major and most likely BREAKING changes, wo. backwards compatibility, or removing temporary backwards compatibility functionality. Use ! or BREAKING:.

See here (internal): https://github.com/equinor/ecalc-internal/discussions/1044

Have you remembered and considered?

  • IF FEAT: I have remembered to update documentation
  • IF FIX OR FEAT: I have remembered to update manual changelog (docs/drafts/next.draft.md)
  • IF BREAKING: I have remembered to update migration guide (docs/docs/migration_guides/)
  • IF BREAKING: I have committed with BREAKING: in footer or ! in header
  • I have added tests (if not, comment why)
  • I have used conventional commits syntax (if you squash, make sure that conventional commit is used)
  • I have included the Github issue nr in the footer!

What is this PR all about?

When a compressor has zero input rate, the compressor train returns create_empty() stage results where inlet_stream = None, and all rate properties (inlet_actual_rate, mass_rate, standard_rate, etc.) return NaN.

During yearly resampling, TimeSeries.resample() uses ffill(). The input rate_sm3_day = 0 survives correctly, but the NaN actual-rate fields get forward-filled with the last positive value — producing a mismatch where standard rate = 0 but inlet actual rate > 0. The bug only manifests with yearly output frequency.

This PR changes 8 rate properties in CompressorTrainStageResultSingleTimeStep to return 0.0 instead of np.nan when the stream is None. Zero flow means zero actual / standard / mass rate — physically correct and resampling-stable. Pressure, density, kappa, z-factor, and temperature still return NaN at zero flow because those quantities are genuinely undefined.

What else did you consider?

Fixing this in the resampling layer instead — but ffill is the right behaviour for genuinely missing samples; the problem is that the source values were never NaN in a meaningful sense. Fixing it at the source is cleaner.

Between the lines?

  • New test: test_zero_rate_gives_zero_actual_rate_not_nan (simplified train)
  • Updated 3 existing test assertions from np.isnan(...) to == 0.0
  • Updated snapshot files (NaN → 0.0 in JSON output)

Resolves equinor/ecalc-internal#1398 (sub-issue of equinor/ecalc-internal#1392).

@olelod olelod force-pushed the fix/zero-rate-actual-rate-ffill branch from 220c8fd to 5e9b82d Compare May 7, 2026 13:56
@olelod olelod marked this pull request as ready for review May 7, 2026 13:58
@olelod olelod requested a review from a team as a code owner May 7, 2026 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant